Worker MultiStreamConsumer#1726
Closed
ibc wants to merge 10 commits into
Closed
Conversation
# Details
- `MultiStreamConsumer` replaces `SimpleConsumer` and `SimulcastConsumer` and merges both.
## TODO
I've renamed `TestSimpleConsumer.cpp` to `TestMultiStreamConsumer.cpp` and it fails miserably. And I don't understand why.
Run it as follows:
```bash
MEDIASOUP_TEST_TAGS="[consumer]" MS_TEST_LOG_LEVEL=debug MS_TEST_LOG_TAGS="rtp simulcast" make test
```
Output:
```
RTC::MultiStreamConsumer::MultiStreamConsumer() | ------ encoding.ssrc:1234567890
RTC::MultiStreamConsumer::CreateRtpStream() | [ssrc:1234567890, payloadType:111]
RTC::MultiStreamConsumer::SendRtpPacket() | ---- 5 packet->GetSsrc():5, mapMappedSsrcSpatialLayer.size():1
RTC::MultiStreamConsumer::SendRtpPacket() | ------ 5.1 <this->mapMappedSsrcSpatialLayer>
RTC::MultiStreamConsumer::SendRtpPacket() | ------------ mappeSsrc:1234567890, spatialLayer:0
RTC::MultiStreamConsumer::SendRtpPacket() | ------ 5.1 </this->mapMappedSsrcSpatialLayer>
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
mediasoup-worker-test is a Catch2 v3.12.0 host application.
Run with -? for options
-------------------------------------------------------------------------------
Scenario: MultiStreamConsumer
RTP packets are not forwarded when the consumer is not active
-------------------------------------------------------------------------------
../../../test/src/RTC/TestMultiStreamConsumer.cpp:232
...............................................................................
../../../test/src/RTC/TestMultiStreamConsumer.cpp:232: FAILED:
{Unknown expression after the reported line}
due to unexpected exception with message:
absl::container_internal::raw_hash_map<>::at
```
Pay attention to these temporal logs I added in `SendRtpPacket()`:
```c++
void MultiStreamConsumer::SendRtpPacket(RTC::RTP::Packet* packet, RTC::RTP::SharedPacket& sharedPacket)
{
MS_TRACE();
#ifdef MS_RTC_LOGGER_RTP
packet->logger.consumerId = this->id;
#endif
// TODO
MS_DUMP(
"---- 5 packet->GetSsrc():%" PRIu32 ", mapMappedSsrcSpatialLayer.size():%zu",
packet->GetSsrc(),
this->mapMappedSsrcSpatialLayer.size());
// TODO
MS_DUMP("------ 5.1 <this->mapMappedSsrcSpatialLayer>");
for (const auto& kv : this->mapMappedSsrcSpatialLayer)
{
MS_DUMP("------------ mappeSsrc:%" PRIu32 ", spatialLayer:%" PRIi16, kv.first, kv.second);
}
MS_DUMP("------ 5.1 </this->mapMappedSsrcSpatialLayer>");
auto spatialLayer = this->mapMappedSsrcSpatialLayer.at(packet->GetSsrc());
// TODO
MS_DUMP("----- 5.2 OK!!!");
```
How the hell is possible that it crashes in `auto spatialLayer = this->mapMappedSsrcSpatialLayer.at(packet->GetSsrc())` line if it's clear that an entry with key (`mappedSsrc: 1234567890`) is present in `this->mapMappedSsrcSpatialLayer`?.
Member
Author
TODO 1STR
|
Member
Build mediasoup with |
Member
Author
Actually no rtc logger rtp related logs ONCE audio consumer is "lost". |
Member
Author
More about TODO 1: Audio consumer issue when there is video as well(Read STR in updated comment TODO 1 above: #1726 (comment)) When the sender only enables audio, the audio Consumer sends all audio packets properly to the consuming browser tab and those are its logs. Everything good AFAIS (I think): When pausing mic Producer: When resuming mic Producer: Now I add webcam Producer in the sender tab (logs related to video are NOT shown, so those logs are just about the existing audio Consumer). BTW nothing to show here since everything works fine, nothing changed Now, with webcam Producer still enabled, I close the mic Producer ( Basically, when there is a video Consumer in the same transport, audio packets are ignored in the audio Consumer. And this happens specially if the video Producer is created first of if the video RTP arrives to mediasoup before the first audio packet (I think). |
|
clang-tidy review says "All clean, LGTM! 👍" |
Member
Author
|
To simplify: the problem is that when there is a video Consumer in same transport and later an audio Consumer is created, Probably we run something that changes |
|
clang-tidy review says "All clean, LGTM! 👍" |
Member
Author
|
I think I found the problem: Working audioThere is only audio Consumer: Notice that Failing audioThere is a video Consumer running. Then an audio Consumer is created and it fails: Notice that |
Member
Author
|
Audio Consumer issue fixed here: 212a2ee |
|
clang-tidy review says "All clean, LGTM! 👍" |
Member
Author
|
I will leave this draft PR open temporary until we have a proper approach so then will close it. |
Member
Author
|
Closing in favour of PR #1731. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
MultiStreamConsumerreplacesSimpleConsumerandSimulcastConsumerand merges both.Things to test / double-check
TestMultiStreamConsumerfails miserably. AFAIS no packet is finally sent, probably due to some checks about current spatial layers and so on.this->encodingContextcan now be nullptr. Check it.this->keyFrameSupported, so also thoseif (!packet->IsKeyFrame())conditions?